Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Viewer support #648

Merged
merged 2 commits into from
Jan 14, 2025
Merged

Add Viewer support #648

merged 2 commits into from
Jan 14, 2025

Conversation

forsyth2
Copy link
Collaborator

@forsyth2 forsyth2 commented Nov 26, 2024 •
edited
Loading

Issue resolution

Select one: This pull request is...

  • a bug fix: increment the patch version
  • a small improvement: increment the minor version
  • an incompatible (non-backwards compatible) API change: increment the major version

@forsyth2 forsyth2 self-assigned this Nov 26, 2024
This was referenced Nov 26, 2024
Merged
Closed
Draft
@forsyth2 forsyth2 marked this pull request as ready for review December 9, 2024 21:58
@forsyth2
Copy link
Collaborator Author

forsyth2 commented Jan 14, 2025 •
edited
Loading

As an example of our new commit paradigm, discussed on E3SM-Project/zstash#355:

On https://github.com/E3SM-Project/zppy/pull/648/commits we initially had 4 commits:
Screenshot 2025-01-14 at 10 26 02 AM

The second commit is really just an extension of the 1st. The 4th commit resets the auto-generated tests that were included in the 3rd commit. So really we can squash these down to 2 commits:

git checkout issue-601-viewers

# This part is unnecessary, but I like to create a backup branch in case I mess something up
git checkout -b issue-601-viewers-until-20250114
git checkout issue-601-viewers # Switch back

git log
# Find last commit before the commits in the PR
git rebase -i 69fe79dd718832f52d7a0fae31c0257b9ee70915
# Change the two extra commits to `f` for fixup (a type of squash)
# pick 37cb030 Add Viewer support
# f 7c5e2b8 Update extra vars and make_viewer
# pick ee60560 Use updated nco for zi input
# f 20aa769 Reset tests

git log
# Now, there are 2 commits instead of 4

After merging #654, we have some merge conflicts to deal with too:
Screenshot 2025-01-14 at 10 35 26 AM

git fetch upstream main
git rebase upstream/main
git status # See the unmerged paths

In zppy/defaults/default.ini:

<<<<<<< HEAD
=======
# Number of columns per page
ncols = integer(default=2)
# Number of rows per page
nrows = integer(default=4)
# Deprecated; legacy name for plots_original; kept for backwards compatibility
# plots_original replaces it with the same default.
# So, if a cfg used the default value before, the behavior will remain the same.
# And if a cfg set plot_names explicitally, then global_time_series.py will override plots_original.
plot_names = string(default="")
>>>>>>> 48d9872 (Add Viewer support)

Change to:

# Number of columns per page
ncols = integer(default=2)
# Number of rows per page
nrows = integer(default=4)

In zppy/templates/global_time_series.bash:

<<<<<<< HEAD
zi-global-time-series --use_ocn {{ use_ocn }} --input {{ input }} --input_subdir {{ input_subdir }} --moc_file {{ moc_file }} --case_dir {{ output }} --experiment_name {{ experiment_name }} --figstr {{ figstr }} --color {{ color }} --ts_num_years {{ ts_num_years }} --plots_original {{ plots_original }} --plots_atm {{ plots_atm }} --plots_ice {{ plots_ice }} --plots_lnd {{ plots_lnd }} --plots_ocn {{ plots_ocn }} --nrows 4 --ncols 2 --results_dir ${results_dir} --regions {{ regions }} --start_yr {{ year1 }} --end_yr {{ year2 }}
=======
zi-global-time-series --use_ocn {{ use_ocn }} --input {{ input }} --input_subdir {{ input_subdir }} --moc_file {{ moc_file }} --case_dir {{ output }} --experiment_name {{ experiment_name }} --figstr {{ figstr }} --color {{ color }} --ts_num_years {{ ts_num_years }} --plots_original {{ plots_original }} --atmosphere_only {{ atmosphere_only }} --plots_atm {{ plots_atm }} --plots_ice {{ plots_ice }} --plots_lnd {{ plots_lnd }} --plots_ocn {{ plots_ocn }} --nrows {{ nrows }} --ncols {{ ncols }} --results_dir ${results_dir} --regions {{ regions }} --make_viewer {{ make_viewer }} --start_yr {{ year1 }} --end_yr {{ year2 }}
>>>>>>> 48d9872 (Add Viewer support)

Change to:

zi-global-time-series --use_ocn {{ use_ocn }} --input {{ input }} --input_subdir {{ input_subdir }} --moc_file {{ moc_file }} --case_dir {{ output }} --experiment_name {{ experiment_name }} --figstr {{ figstr }} --color {{ color }} --ts_num_years {{ ts_num_years }} --plots_original {{ plots_original }} --plots_atm {{ plots_atm }} --plots_ice {{ plots_ice }} --plots_lnd {{ plots_lnd }} --plots_ocn {{ plots_ocn }} --nrows {{ nrows }} --ncols {{ ncols }} --results_dir ${results_dir} --regions {{ regions }} --make_viewer {{ make_viewer }} --start_yr {{ year1 }} --end_yr {{ year2 }}

(Remove --atmosphere_only {{ atmosphere_only }}, keep --nrows {{ nrows }} --ncols {{ ncols }} and --make_viewer {{ make_viewer }})

git grep -n "<<<" zppy
# No more diffs
git add zppy/defaults/default.ini zppy/templates/global_time_series.bash
git rebase --continue

@forsyth2
Copy link
Collaborator Author

#654 removed scratch, atmosphere_only, and plot_names. None of these occur on https://github.com/E3SM-Project/zppy/pull/648/files.

@forsyth2 forsyth2 merged commit 57c0891 into main Jan 14, 2025
4 checks passed
@forsyth2 forsyth2 deleted the issue-601-viewers branch January 14, 2025 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@forsyth2 forsyth2

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature]: Create HTML page for global time series
1 participant
@forsyth2